Make MarkRevealed and RecordWonBid order-independent - #170
Open
damilolaedwards wants to merge 1 commit into
Open
Make MarkRevealed and RecordWonBid order-independent#170damilolaedwards wants to merge 1 commit into
damilolaedwards wants to merge 1 commit into
Conversation
MarkRevealed (fired by RevealService's own gate/timer) and RecordWonBid (fired by InclusionTracker's head-event loop) are driven by independent goroutines with no happens-before edge between them. Whenever a head event is delayed past the reveal gate, MarkRevealed can run first: it found no pending entry, silently skipped the balance deduction, and the RecordWonBid that arrived afterward created a pending entry that was never marked revealed - orphaned until it expired two epochs later, understating the effective balance for the whole window. PaymentTracker now records a slot in a small earlyReveals set when MarkRevealed can't find a pending entry, instead of just giving up. When RecordWonBid later runs for that slot, it applies the deduction immediately and skips creating a pending entry, since the bid was already revealed. Stale early-reveal markers (a reveal recorded but the matching won bid report never arrives) are cleared by the same two-epoch prune pass that already clears expired pending payments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
MarkRevealed (fired by RevealService's own gate/timer) and RecordWonBid
(fired by InclusionTracker's head-event loop) are driven by independent
goroutines with no happens-before edge between them. Whenever a head event
is delayed past the reveal gate, MarkRevealed can run first: it found no
pending entry, silently skipped the balance deduction, and the RecordWonBid
that arrived afterward created a pending entry that was never marked
revealed - orphaned until it expired two epochs later, understating the
effective balance for the whole window.
Fix
PaymentTracker now records a slot in a small earlyReveals set when
MarkRevealed can't find a pending entry, instead of just giving up. When
RecordWonBid later runs for that slot, it applies the deduction immediately
and skips creating a pending entry, since the bid was already revealed.
Stale early-reveal markers (a reveal recorded but the matching won bid
report never arrives) are cleared by the same two-epoch prune pass that
already clears expired pending payments.
Testing
TestPaymentTracker_MarkRevealedBeforeRecordWonBidreproduces the exactrace and asserts the deduction now lands correctly with nothing left
pending;
TestPaymentTracker_RecordWonBidBeforeMarkRevealedconfirms thecommon ordering is unaffected;
TestPaymentTracker_EarlyRevealNeverRecordedIsPrunedconfirms the leak-prevention path.
Verified the main regression test actually catches the bug: reverted just
the production file, ran the test, and it failed with exactly the predicted
numbers (balance adjustment stuck at 0 instead of -5000, 5000 stuck in
pending instead of 0), then restored the fix and confirmed it passes.
go build,go vet, andgo test -race ./pkg/...all pass (aside from apre-existing, unrelated failure in
pkg/webuicaused by the frontend notbeing built in this checkout).